Skip to content

fix(auth): make logout revoke, and keep the JWT out of the URL - #72

Merged
mariuspruvot merged 1 commit into
mainfrom
fix/session-revocation
Aug 1, 2026
Merged

fix(auth): make logout revoke, and keep the JWT out of the URL#72
mariuspruvot merged 1 commit into
mainfrom
fix/session-revocation

Conversation

@mariuspruvot

Copy link
Copy Markdown
Owner

The two identity findings that had been open since the first audit round.

Logout did not log anyone out

POST /auth/logout deleted the cookie and returned {"status": "ok"}. A refresh token copied out of the browser — which is the only scenario where logging out matters — stayed valid for its full seven days. The endpoint was a no-op against exactly the person it existed to stop.

A JWT cannot be withdrawn once issued, so each refresh token now carries the ver it was minted under, and GitHubUser.token_version is bumped on logout. Cheaper than a denylist: nothing stored per token, nothing to expire, no cleanup job.

Two consequences worth agreeing on before merge:

  1. It logs the user out everywhere, not just in this browser. For a tool holding a GitHub OAuth token and a Claude subscription credential, that is what someone clicking logout after losing a laptop expects. A per-session jti denylist would be narrower, at the cost of a table and an expiry job.
  2. Tokens issued before this migration are rejected, since they carry no ver claim — current users get logged out once on deploy. That is the intent rather than a side effect: the point is that tokens minted under the old rules stop being honoured.

POST /auth/logout is authenticated now, because revocation has to know whose tokens to invalidate.

The access token was in the redirect URL

RedirectResponse(url=f"{settings.APP_BASE_URL}/auth/callback?access_token={tokens.access_token}")

That URL lands in browser history, in the Referer of whatever the page loads next, and in every proxy log between the two — carrying the credential that authenticates the whole API.

The redirect carries no token now. It sets the httpOnly refresh cookie, and the frontend trades that for an access token by calling POST /auth/refresh — through refreshToken(), which the API client already had for silent renewal and which is now exported rather than re-implemented in the callback.

OAuthCallback.tsx no longer reads useSearchParams at all, which is the property the new test pins: the token can only come from the cookie exchange.

Verification

  • 426 backend tests (was 422) and 66 frontend tests; ruff, ruff format, mypy, tsc --noEmit and eslint all clean
  • New backend coverage: a token issued before logout is rejected, a token with no ver claim is rejected, a freshly issued one still works after revocation, logout is 401 without auth, logout increments the version
  • The callback test asserts the redirect has no access_token= and does set the refresh cookie
  • Migration 5b500a24f018 autogenerated against a real Postgres and applied over the full chain

Note

docs/self-hosting.md needs no change — nothing there documented the callback's query string. CLAUDE.md gained both invariants.

Logging out only deleted the cookie. A refresh token copied out of the
browser stayed valid for its full seven days, so the endpoint did nothing
at all against the one person it was meant to stop.

A JWT cannot be withdrawn once issued, so each refresh token now carries the
`ver` it was minted under and `GitHubUser.token_version` is bumped on
logout. Cheaper than a denylist: nothing stored per token, nothing to
expire. It invalidates all of the user's sessions rather than just this
browser's, which for a tool holding GitHub and Claude credentials is the
behaviour someone clicking logout after losing a laptop expects. Logout is
authenticated now, because revocation has to know whose tokens to kill.

Tokens issued before the column existed carry no `ver` claim and are
rejected the same way, logging current users out once. That is the intent,
not a side effect.

Separately, the OAuth callback put the access token in the redirect URL,
where it landed in browser history, in the Referer of whatever the page
loaded next, and in every proxy log along the way -- for the credential that
authenticates the entire API. The redirect carries no token now: it sets the
httpOnly refresh cookie, and the frontend trades that for an access token
through the `refreshToken()` helper the API client already had for silent
renewal.

426 backend tests, 66 frontend.
@helprs-prod

helprs-prod Bot commented Aug 1, 2026

Copy link
Copy Markdown

helPRs session created for this PR.

Skill: challenge-me | Open session

@mariuspruvot
mariuspruvot merged commit 070111e into main Aug 1, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant